Skip to content

Fix/socket collaboration migration - #1342

Merged
vijaypatil477 merged 4 commits into
mainfrom
fix/socket-collaboration-migration
Aug 2, 2026
Merged

Fix/socket collaboration migration#1342
vijaypatil477 merged 4 commits into
mainfrom
fix/socket-collaboration-migration

Conversation

@vijaypatil477

@vijaypatil477 vijaypatil477 commented Aug 1, 2026

Copy link
Copy Markdown
Owner

✦ Description

Please include a clear and concise summary of the changes made and the problem solved.

Fixes # (issue number)

⟡ Type of Change

Please delete options that are not relevant:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update (non-breaking change to docs)
  • Code styling/formatting (prettier, eslint, spacing)

✦ Checklist

  • My code follows the style guidelines of this project.
  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • My changes generate no new warnings or console errors.
  • I have verified that my changes work correctly on both desktop and mobile viewports.
  • (If applicable) I have run npm run lint and npm run format locally before pushing.

⟡ Screenshots / Screen Recordings (Required for UI changes)

Please provide Before and After screenshots or a short GIF showcasing the visual changes.

Before After
Insert image Insert image

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas

Summary by CodeRabbit

  • New Features

    • Added real-time room collaboration with synchronized code, input, language, cursors, presence, roles, and chat.
    • Added room creation support with improved connection handling.
    • Added a fallback code-execution provider when the primary service is unavailable.
  • Bug Fixes

    • Improved execution-result caching to avoid storing failed or infrastructure-error results.
    • Signup can continue when username verification temporarily fails.
    • Improved WebRTC signal handling and room cleanup reliability.
  • Tests

    • Added browser-oriented test configuration and updated CORS expectations.

@vercel

vercel Bot commented Aug 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
debugra Ready Ready Preview Aug 2, 2026 4:31am

@github-actions github-actions Bot added quality:exceptional Exceptional code quality contribution type:bug Vulnerability or logical bug fixes type:performance Performance and latency optimizations type:testing Integration and unit test coverage labels Aug 1, 2026
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

Hi @vijaypatil477, thanks for contributing to Debugra! 🎉

I have automatically:

  • 👤 Assigned this PR to you.
  • 🏷️ Applied the gssoc:approved label.

Our workflows will now analyze your changes to classify:

  • 📈 PR Difficulty: level:*
  • 🧩 PR Type: type:*
  • 🌟 PR Quality: quality:*

Tip

Ensure your PR description references the issue it resolves (e.g. Closes #123). This allows the bot to inherit any additional labels from that issue!

Happy coding! 🚀

@github-actions github-actions Bot added gssoc:approved GSSoC '26 Approved issue level:critical GSSoC '26 Critical difficulty issue labels Aug 1, 2026
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@omkhandare55, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 58 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c89fe57a-8b5d-4acb-9baa-9dfd4f25d12b

📥 Commits

Reviewing files that changed from the base of the PR and between e82e9bf and 3f67b77.

📒 Files selected for processing (3)
  • server/server.js
  • src/components/Editor/EditorPage.jsx
  • vite.config.js
📝 Walkthrough

Walkthrough

The change adds Socket.IO-based room collaboration, modular Firebase Admin access, broader Firestore rules, Judge0-to-Wandbox execution fallback, updated webhook and rate-limit behavior, and ESLint, Vitest, and Vite configuration.

Changes

Realtime collaboration

Layer / File(s) Summary
Room data and Firebase access
firestore.rules, server/services/firebaseAdmin.js, server/services/roomCleanupService.js, server/routes/rooms.js
Firebase uses modular initialization. Room creation uses the shared Firestore handle. Firestore rules allow authenticated room access and public profile reads.
Socket.IO server lifecycle
server/server.js, server/services/socketService.js, server/package.json
The application runs on an HTTP server with Socket.IO. The socket service manages room state, presence, collaboration events, and final persistence.
Client room synchronization and chat
src/hooks/useRoom.js, src/components/Chat/ChatPanel.jsx, src/components/Editor/EditorPage.jsx, package.json
Room synchronization and chat use Socket.IO events instead of Firestore listeners and writes.
WebRTC listener handling
src/hooks/useWebRTC.js
Participant and signal listener errors are logged. Processed signal documents remain in Firestore.
Signup validation handling
src/components/Auth/AuthModal.jsx
Signup continues after username lookup errors and stops for duplicate usernames.

Execution and platform behavior

Layer / File(s) Summary
Judge0 fallback and result caching
server/services/judge0Service.js, server/routes/execute.js
Code execution validates languages, tries Judge0 first, falls back to Wandbox, and caches only accepted results without OCI runtime errors.
HTTP middleware and webhook responses
server/middleware/rateLimiter.js, server/routes/webhooks.js
Rate-limit keys use the full request object. Webhook requests return success when no destination is configured.

Development tooling

Layer / File(s) Summary
Lint, test, and build configuration
eslint.config.js, vitest.config.js, vite.config.js, package.json
ESLint 9 flat configuration and Vitest configuration are added. Vite receives Node polyfills, Monaco externalization, and updated COOP headers.
Project maintenance
.gitignore, server/package.json, server/tests/cors.test.js
Ignore rules and dependency versions are updated. The CORS test reflects origin-less preflight rejection.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Editor as EditorPage
  participant Room as useRoom
  participant Socket as Socket.IO server
  participant Store as Firestore
  Editor->>Room: connect to room
  Room->>Socket: join room and send editor events
  Socket->>Store: seed or persist room state
  Socket-->>Room: state, presence, cursor, and chat events
  Room-->>Editor: update editor and collaboration state
Loading

Possibly related PRs

Suggested labels: type:refactor

Poem

A rabbit hops through rooms of code,
While sockets share the editor load.
Cursors move and chat lights glow,
Judge0 falls back when engines slow.
Lint and tests guard every trail.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description leaves the summary, issue reference, change type, checklist, and required UI screenshots incomplete. Add a concise change summary, issue number, applicable change type, completed checklist items, and screenshots for the UI changes.
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: migrating collaboration functionality to Socket.IO.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch fix/socket-collaboration-migration
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/socket-collaboration-migration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/Editor/EditorPage.jsx (1)

1943-1961: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add a key so ChatPanel resets when the room changes.

ChatPanel holds messages in local state and no longer loads them from Firestore. A room switch changes the roomId and socket props but does not remount the component, so React keeps the previous room's messages array. The user then sees messages from the previous room in the new room.

Set key={room.roomId} on both instances to force a fresh mount per room.

🐛 Proposed fix
         <ChatPanel
+          key={room.roomId}
           roomId={room.roomId}
           user={user}
           isOpen={true}
           onToggle={() => setMobileTab(MOBILE_TABS.CODE)}
           socket={room.socket}
         />
       <ChatPanel
+        key={room.roomId}
         roomId={room.roomId}
         user={user}
         isOpen={chatOpen}
         onToggle={() => setChatOpen(!chatOpen)}
         socket={room.socket}
       />
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Editor/EditorPage.jsx` around lines 1943 - 1961, Update both
ChatPanel instances in EditorPage so each receives key={room.roomId}, ensuring
switching rooms remounts ChatPanel and resets its local messages state.
🟠 Major comments (15)
src/hooks/useWebRTC.js-100-116 (1)

100-116: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Prevent replay of processed signal documents.

Firestore sends existing matching documents as added when this listener starts. The handler only checks peersRef.current[data.senderUid], so an unprocessed signal from before a disconnect can be applied to the new peer. Restore one-time consumption or add a call-session/document ID and consume/deduplicate signals once before calling peer.signal().

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks/useWebRTC.js` around lines 100 - 116, Update the signal handling in
useWebRTC’s onSnapshot callback to consume or deduplicate each signal document
before creating a peer or calling peer.signal(). Ensure documents replayed as
added on listener startup, including stale pre-disconnect signals, are ignored
after prior processing while allowing each current call-session signal to be
applied once.
eslint.config.js-24-30 (1)

24-30: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Register Vitest globals for src/hooks/__tests__ files.

Vitest includes src/**/__tests__/**/*.{test,spec}.{js,jsx} and enables globals, but these imports use describe, it, expect, and vi directly. Add a later flat-config entry for src/hooks/__tests__ that sets languageOptions.globals.vitest.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@eslint.config.js` around lines 24 - 30, Update the flat ESLint configuration
after the existing config entry with a later rule targeting src/hooks/__tests__
test files, and set languageOptions.globals.vitest for that pattern so direct
describe, it, expect, and vi usage is recognized.
vite.config.js-20-26 (1)

20-26: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not externalize Monaco in the browser build.

external leaves monaco-editor/esm/vs/editor/editor.api as a bare browser import. @monaco-editor/react and monaco-vim need Monaco resolved by the bundler; with this config, the Editor route can fail in production. Remove the rolldownOptions.external entry and add monaco-editor directly if peer resolution fails.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@vite.config.js` around lines 20 - 26, Update the build configuration’s
rolldownOptions by removing the external entry for
monaco-editor/esm/vs/editor/editor.api so Monaco is bundled for browser
production builds; if peer dependency resolution still fails, add monaco-editor
directly to the build inputs or dependencies using the existing project
convention.
server/services/judge0Service.js-3-29 (1)

3-29: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Honor custom JUDGE0_API_URL values.

getJudge0Config returns either the RapidAPI endpoint or https://ce.judge0.com/submissions?wait=true; it never uses envUrl for non-RapidAPI values. A configured self-hosted Judge0 URL is discarded, and code execution then posts source code to the public Judge0 CE endpoint. Add a branch that uses envUrl for custom, non-RapidAPI URLs unless that configuration option is no longer supported.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/services/judge0Service.js` around lines 3 - 29, Update getJudge0Config
so configured non-RapidAPI envUrl values are preserved instead of falling back
to the public Judge0 endpoint. Add a custom-URL branch after the RapidAPI
handling, returning envUrl with the required submissions?wait=true path and JSON
headers, while retaining the existing RapidAPI and default public endpoint
behavior.
server/middleware/rateLimiter.js-4-5 (1)

4-5: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Return the actual client IP from getIpKey.

ipKeyGenerator(req) does not create an IP-based limiter key. getIpKey should fall back to a client IP, with IPv6 normalized via ipKeyGenerator(req.ip), otherwise the rate limiter can get one bucket per request object and allow clients to bypass max: 5.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/middleware/rateLimiter.js` around lines 4 - 5, Update getIpKey to
derive the key from the client IP rather than passing the request object to
ipKeyGenerator: use req.ip as the fallback input and normalize it through
ipKeyGenerator(req.ip), preserving the actual client-IP-based limiter bucket.
src/hooks/useRoom.js-136-147 (1)

136-147: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

join-room always reports the stale initial role.

Line 145 sends role: myRole. myRole derives from roomData (Lines 107-109), and roomData is loaded by the separate effect at Lines 202-216, which resolves after the socket connects.

At connect time roomData is null, so myRole evaluates to 'viewer' for every user, including the room host. The effect deps at Line 185 omit myRole, and join-room is emitted only inside the connect handler, so the role is never corrected. The presence list built at socketService.js Lines 68-73 therefore shows every collaborator as a viewer for the whole session.

Emit a role update after roomData resolves, or wait for roomData before joining.

🐛 Proposed fix

Send the role in a separate effect that reacts to myRole:

+  // Re-announce the role whenever it resolves or changes.
+  useEffect(() => {
+    if (!roomId || !user || !socket || !isConnected) return;
+    socket.emit('join-room', {
+      roomId,
+      user: {
+        uid: user.uid,
+        displayName: user.displayName || user.email?.split('@')[0] || 'Guest',
+        email: user.email,
+      },
+      role: myRole,
+    });
+  }, [roomId, user, socket, isConnected, myRole]);

The server treats join-room as idempotent (socketService.js Lines 67-79), so the repeat call updates the existing entry. The server must still resolve the authoritative role itself, as noted on server/services/socketService.js.

Also applies to: 185-185

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks/useRoom.js` around lines 136 - 147, Update the socket join flow
around the connect handler and the effect at line 185 so room membership is
emitted again when myRole changes after roomData loads. Keep the initial
connection behavior, but add a role-reactive join-room emission using the
existing payload and socket state, ensuring the server receives the
authoritative resolved role instead of retaining the initial viewer value.
firestore.rules-56-58 (1)

56-58: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Any signed-in user can delete other users' subcollection documents.

votes, signals, and drawings now allow delete for any authenticated user, with no participant check and no owner check. create is also unconstrained, so a document is not tied to its author.

A signed-in user who is not a participant can delete another room's whiteboard drawings and WebRTC signal documents, which breaks the call setup and the whiteboard for that room.

Restrict these rules to room participants, and restrict delete to the author or the intended recipient.

🔒 Proposed direction
+      function isParticipant() {
+        let room = get(/databases/$(database)/documents/rooms/$(roomId)).data;
+        return request.auth != null
+               && (request.auth.uid == room.createdBy || request.auth.uid in room.participantIds);
+      }
+
       match /signals/{signalId} {
-        allow read, create, delete: if request.auth != null;
+        allow read: if isParticipant();
+        allow create: if isParticipant() && request.resource.data.senderId == request.auth.uid;
+        allow delete: if isParticipant()
+                      && (resource.data.senderId == request.auth.uid
+                          || resource.data.recipientId == request.auth.uid);
       }
 
       match /drawings/{drawingId} {
-        allow read, create, delete: if request.auth != null;
+        allow read, create: if isParticipant();
+        allow delete: if isParticipant() && resource.data.uid == request.auth.uid;
       }

Adjust the field names to match the documents that src/hooks/useWebRTC.js and the whiteboard component write.

Also applies to: 84-91

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@firestore.rules` around lines 56 - 58, Update the Firestore rules for the
affected subcollections around the `allow read/create/update/delete` statements
so access requires room participation, creation binds the document to its author
using the field names written by `useWebRTC.js` and the whiteboard component,
and deletion is limited to that author or the intended recipient. Apply the same
restrictions to the additional affected rule block.
src/hooks/useRoom.js-356-361 (1)

356-361: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

The execution vote no longer reaches a real quorum.

Presence moved to Socket.IO, so activeUsers is no longer written to the room document. This join path updates participantIds and roles only.

castVote still reads presence from Firestore: const totalUsersCount = (data.activeUsers || []).length; at Line 504. That value is now always 0. The initiator pre-approves at Line 459, so approvals.length > 0 / 2 is true immediately and the vote status becomes approved on creation. The consensus gate on code execution is bypassed. The deadlock-resolution effect at Line 607 has the same problem.

Make the active-user count available to the vote logic. Two options:

  • Have the server write the current activeUsers count into the room document on each presence-update, so the transaction reads a fresh value.
  • Pass the socket-derived activeUsers.length into castVote and store it on the vote when it is created.

The first option is safer, because a client-supplied count lets any client force a quorum.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks/useRoom.js` around lines 356 - 361, Update the Socket.IO
presence-update handling so the server persists the current active-user count in
the room document, preserving the existing activeUsers field shape consumed by
castVote and the deadlock-resolution effect. Ensure the write occurs whenever
presence changes and that vote creation and quorum checks read this fresh
server-derived value instead of defaulting to an empty list.
src/components/Chat/ChatPanel.jsx-46-66 (1)

46-66: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Chat messages are no longer persisted, so history is lost.

This component now receives messages only from the live socket. server/services/socketService.js Lines 142-146 relays chat-message without storing it, and nothing else writes the messages subcollection.

Consequences:

  • A user who joins after a message was sent never sees it.
  • A page reload clears the entire conversation.
  • Messages sent while a user's socket is down are lost permanently, because the server relays with socket.to(roomId) and keeps no buffer.

firestore.rules Lines 68-70 still grant read, create on rooms/{roomId}/messages, which indicates the collection is still expected to hold the history.

Write each message to Firestore from the server when it relays the event, and load the recent history when the panel mounts.

Do you want me to open an issue for restoring chat persistence?

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Chat/ChatPanel.jsx` around lines 46 - 66, Restore chat
persistence by updating the server’s chat-message relay in socketService to
write each message to the room’s Firestore messages subcollection before or
alongside broadcasting it. In ChatPanel’s mount effect, load recent messages for
roomId from Firestore, initialize messages with that history, and retain the
existing socket listener for new-message updates and deduplication.
server/services/socketService.js-5-7 (1)

5-7: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Room state survives only in process memory and is flushed only when the room empties.

activeRooms is module-level state in a single Node process, and Firestore receives a write only when activeUsers.length === 0 (Line 165).

Two failure modes follow:

  • A crash, a deploy, or a restart loses every unsaved edit for every active room. The previous Firestore-backed sync wrote continuously, so this migration removes durability that users relied on.
  • With more than one server instance, two users in the same room can connect to different instances. Each instance keeps its own activeRooms entry, so the users never see each other's edits. The instance that empties last overwrites the other instance's saved code.

Recommended changes:

  • Flush room.code, room.language, room.stdin, and updatedAt on a debounced interval, for example every 10 seconds when room.code !== room.lastSavedCode. lastSavedCode is already tracked at Line 60 but never read.
  • Add the Socket.IO Redis adapter and move activeRooms to shared storage before running more than one instance.
  • Flush all rooms on SIGTERM before the process exits.

Do you want me to implement the debounced flush and the SIGTERM handler?

Also applies to: 164-180

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/services/socketService.js` around lines 5 - 7, Make room persistence
durable by periodically flushing each activeRooms entry when room.code differs
from lastSavedCode, including code, language, stdin, and updatedAt, then update
lastSavedCode after a successful write. Add a SIGTERM shutdown handler that
flushes all rooms before exit, and configure the Socket.IO Redis adapter with
shared room-state storage so users connected to different instances observe the
same edits.
server/services/socketService.js-149-182 (1)

149-182: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

One disconnected tab removes a user who is still connected elsewhere.

currentRoomId and currentUser are per-socket closures, but activeUsers is deduplicated by uid at Line 67. If one user opens two tabs, the room holds a single entry for that uid.

When the first tab disconnects, Line 159 filters the uid out of activeUsers. The second tab is still joined, but presence reports the user as gone. If that user was the only member, Line 165 treats the room as empty, flushes it, and calls activeRooms.delete(roomId) at Line 179. The surviving socket keeps emitting code-update, but Line 95 returns early because activeRooms no longer holds the room. That tab silently stops syncing until the user reloads.

Track connections per uid, and remove the user only when the last socket for that uid disconnects.

A second problem in the same handler: a socket can call join-room again with a different roomId. Line 28 overwrites currentRoomId, but the socket never leaves the previous Socket.IO room, so it keeps receiving events from it and is never removed from its activeUsers list.

🐛 Proposed direction
     socket.on('join-room', async ({ roomId, user, role }) => {
       if (!roomId || !user) return;
+      if (currentRoomId && currentRoomId !== roomId) {
+        socket.leave(currentRoomId);
+        removeSocketFromRoom(currentRoomId, currentUser, socket.id);
+      }

Store sockets: Set<socketId> on each activeUsers entry. On disconnect, delete socket.id from that set and remove the user only when the set is empty.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/services/socketService.js` around lines 149 - 182, Update the room
membership flow around join-room and the disconnect handler to track socket IDs
per active user, adding and removing socket.id from each user’s sockets Set and
removing the user only when that Set becomes empty. Before switching rooms,
remove the socket from its previous room’s membership and Socket.IO room so
rejoining cannot leave stale presence or subscriptions; preserve the existing
empty-room persistence and cleanup only after the last member is gone.
server/services/socketService.js-94-114 (1)

94-114: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Cap the size of code and stdin payloads.

Neither handler limits the payload size. room.code and room.stdin accept unbounded strings from a client, retain them in the activeRooms map, and broadcast them to every room member.

A single client can push megabytes per event. That grows the process heap without bound and multiplies outbound bandwidth by the number of room members. src/hooks/useRoom.js Lines 540-548 already cap execution output at 10 000 characters, so the codebase has a precedent for this limit.

A payload above the 1 MiB Firestore document limit also makes the persistence write at Line 169 fail permanently, so the room can never be saved again.

Reject or truncate oversized payloads, and set maxHttpBufferSize on the Socket.IO server.

🛡️ Proposed fix
+const MAX_CODE_BYTES = 500 * 1024; // stays under the 1 MiB Firestore document limit
+const MAX_STDIN_BYTES = 64 * 1024;
+
 function initSocketServer(server, allowedOrigins) {
   const io = new Server(server, {
+    maxHttpBufferSize: 1e6,
     cors: { ... },
   });
     socket.on('code-update', ({ roomId, code, language }) => {
       if (!roomId || !activeRooms.has(roomId)) return;
+      if (typeof code !== 'string' || Buffer.byteLength(code, 'utf8') > MAX_CODE_BYTES) {
+        logger.warn(`[Socket] Rejected oversized code-update for room ${roomId}`);
+        return;
+      }

Apply the equivalent guard with MAX_STDIN_BYTES in stdin-update.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/services/socketService.js` around lines 94 - 114, Cap incoming code
and stdin payloads before storing or broadcasting them: define and reuse
byte-size limits, including the proposed MAX_STDIN_BYTES guard, and reject or
truncate values exceeding the limits. Update both the code-update and
stdin-update handlers in the socket service, and configure the Socket.IO
server’s maxHttpBufferSize to enforce the transport-level limit while preserving
normal room updates.
firestore.rules-11-14 (1)

11-14: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Remove the public read allow on /users/{userId}.

The broader rule permits unauthenticated reads on every user document because it matches above the owner-only rule. User documents store uid, displayName, and email, so this exposes PII. The required display-name uniqueness check can use a public displayNames/{normalizedName} collection or a server-side Admin SDK check instead of public reads of user documents.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@firestore.rules` around lines 11 - 14, Remove the `allow read: if true` rule
from the `/users/{userId}` match block, preserving the existing owner-only
access rule. Update signup display-name uniqueness checks to use the
`displayNames/{normalizedName}` collection or a server-side Admin SDK lookup
rather than reading user documents publicly.
src/hooks/useRoom.js-246-296 (1)

246-296: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Store password materials when creating password-protected rooms.

createRoom sets passwordProtected: true, but neither the Firestore write nor /api/rooms/create stores passwordHash or passwordSalt; Firestore rules only block client writes to those fields. Joined users then reach POST /api/rooms/verify-password, which returns 500 Room configuration error. Derive the hash with a fresh salt over HTTPS and write passwordHash and passwordSalt using the Admin SDK.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks/useRoom.js` around lines 246 - 296, Update createRoom to derive a
password hash and fresh salt when trimmedPassword is present, then include
passwordHash and passwordSalt in the server room-creation payload over HTTPS; do
not rely on the client-side setDoc for these protected fields, since they must
be written through the Admin SDK. Ensure password-protected rooms persist both
materials and unprotected rooms retain the existing behavior.
src/components/Auth/AuthModal.jsx-75-86 (1)

75-86: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Fail closed when display-name lookup fails.

If the duplicate lookup throws, this code logs and then runs createUserWithEmailAndPassword, so signup can bypass the read. Client-side Firestore reads are not atomic under match /users/{userId} { allow read: if true; }, and the server has no displayed uniqueness reservation for displayNameLower. Block signup when the lookup cannot complete, or use a trusted atomic reservation.

Proposed fail-closed handling
         } catch (err) {
-          console.warn('[AuthModal] Username uniqueness check failed:', err.message);
+          toast.error('Could not verify the display name. Please try again.');
+          return;
         }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Auth/AuthModal.jsx` around lines 75 - 86, Update the username
uniqueness-check catch block in AuthModal’s signup flow to fail closed: after
logging the lookup error, stop signup, clear the loading state, and surface an
appropriate error instead of continuing to create the account. Preserve the
existing duplicate-name rejection behavior and ensure
createUserWithEmailAndPassword runs only when the lookup completes successfully.
🟡 Minor comments (6)
server/tests/cors.test.js-15-18 (1)

15-18: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Import the Vitest globals in server/tests/cors.test.js.

The lint config only defines browser/ES globals and server/tests/cors.test.js does not explicitly import it, expect, or describe. Add the import above the describe block to avoid these undefined-globals errors.

Proposed fix
+import { expect, it } from 'vitest';
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/tests/cors.test.js` around lines 15 - 18, Import the Vitest globals
used by server/tests/cors.test.js—describe, it, and expect—before the existing
describe block so the test file passes lint without relying on implicit globals.

Source: Linters/SAST tools

server/services/judge0Service.js-6-7 (1)

6-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix no-undef for process in ESLint’s server rules.

eslint.config.js declares globals.browser and globals.es2020 for **/*.{js,jsx}, but not globals.node. Add globals.node or an equivalent Node environment for the server/ files so process is not flagged by no-undef.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/services/judge0Service.js` around lines 6 - 7, Update the ESLint
configuration for server-side JavaScript files to include the Node globals
environment, ensuring process in judge0Service.js and other server files is
recognized and no longer triggers no-undef. Preserve the existing browser and
ES2020 globals configuration for non-server files.

Source: Linters/SAST tools

src/components/Chat/ChatPanel.jsx-80-81 (1)

80-81: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use crypto.randomUUID() for the message id.

Math.random().toString(36).substring(2, 9) produces at most 7 base-36 characters, and fewer whenever toString(36) drops trailing zeros. That is roughly 36^7 values at best, generated independently by every client.

The id serves as the React key at Line 387 and as the deduplication key at Line 52. A collision between two users silently discards a legitimate message.

src/hooks/useRoom.js Line 252 already uses crypto.randomUUID(). The comment on Line 80 also calls this a "UUID prefix", which does not match the implementation.

♻️ Proposed fix
-    // Generate standard client-side UUID prefix
-    const msgId = Math.random().toString(36).substring(2, 9);
+    const msgId =
+      typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function'
+        ? crypto.randomUUID()
+        : `${Date.now()}-${user.uid}-${Math.random().toString(36).slice(2)}`;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Chat/ChatPanel.jsx` around lines 80 - 81, Update the
message-id assignment in ChatPanel to use crypto.randomUUID() instead of the
Math.random-based value, preserving the resulting id’s use as the React key and
deduplication key.
src/hooks/useRoom.js-173-178 (1)

173-178: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Remote cursors are never removed when a user leaves.

The cursor-move handler only adds and updates entries. No handler deletes an entry, and presence-update (Line 160) does not prune remoteCursors.

src/components/Editor/EditorPage.jsx Lines 592-638 renders one decoration and one injected <style> element per entry. Its cleanup at Lines 645-654 removes style elements only for uids that are absent from remoteCursors, and that object never shrinks. A user who leaves the room keeps a visible ghost cursor and a <style> element in the document head until the page reloads.

Prune remoteCursors against the presence list.

🐛 Proposed fix
     newSocket.on('presence-update', (users) => {
       setActiveUsers(users);
+      const liveUids = new Set(users.map((u) => u.uid));
+      setRemoteCursors((prev) => {
+        const next = {};
+        for (const [uid, cursor] of Object.entries(prev)) {
+          if (liveUids.has(uid)) next[uid] = cursor;
+        }
+        return next;
+      });
     });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks/useRoom.js` around lines 173 - 178, Update the presence-update
handling in useRoom so remoteCursors is pruned against the current presence
list, removing entries for users who are no longer present while preserving
active cursor entries. Keep the existing cursor-move updates in the newSocket
handler unchanged.
package.json-11-11 (1)

11-11: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Extend lint to cover server/ with a CommonJS Node config.

lint only runs eslint src, so the new server files never get lint coverage. If server/**.js files are added, ESLint will still apply the existing files: ['**/*.{js,jsx}'] config, which sets sourceType: 'module', browser globals, and React rules. Add a later config block with files: ['server/**/*.js'], sourceType: 'commonjs', and globals: { node: true }, then update the script to include server while excluding the React JSX/client pattern.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@package.json` at line 11, Extend the lint script to lint server files
alongside src while excluding the React JSX/client pattern, and add a later
ESLint configuration block targeting server/**/*.js with CommonJS source type
and Node globals. Preserve the existing client/React configuration for src and
ensure the server override takes precedence.

Source: Linters/SAST tools

src/components/Auth/AuthModal.jsx-77-79 (1)

77-79: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Use one canonical display name for lookup and storage.

Line 78 trims name for the query, but Line 89 passes the raw name to updateProfile. saveUser then derives displayNameLower from that value at Lines 36-38. An input such as " Alice " can store " alice " while a later lookup searches "alice".

Trim the value once. Use it for the query, updateProfile, and displayNameLower.

Proposed normalization
+        const normalizedName = name.trim();
         try {
           const { collection, query, where, getDocs } = await import('firebase/firestore');
           const q = await getDocs(
-            query(collection(db, 'users'), where('displayNameLower', '==', name.trim().toLowerCase()))
+            query(collection(db, 'users'), where('displayNameLower', '==', normalizedName.toLowerCase()))
           );
...
-        await updateProfile(result.user, { displayName: name });
+        await updateProfile(result.user, { displayName: normalizedName });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Auth/AuthModal.jsx` around lines 77 - 79, Normalize the
display name once in the AuthModal submission flow, then reuse that trimmed
value for the users query, updateProfile call, and saveUser/displayNameLower
derivation. Update the relevant variable and references near getDocs and
updateProfile so inputs such as surrounding whitespace produce one canonical
stored and queried name.
🧹 Nitpick comments (5)
server/services/judge0Service.js (2)

86-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Log the fallback when Judge0 returns an unexpected response shape.

If data.status.id is undefined, execution falls through to the Wandbox fallback without any log message. The catch block below logs a warning, so this specific fallback path is comparatively silent. Add a log statement before falling through, so operators can distinguish "Judge0 unreachable" from "Judge0 returned an unexpected payload."

🔍 Proposed fix to log malformed Judge0 responses
     const data = response.data;
     if (data && data.status && data.status.id !== undefined) {
       const stdout = (data.stdout || '').slice(0, MAX_OUTPUT_LENGTH);
       const stderr = (data.stderr || data.compile_output || data.message || '').slice(0, MAX_OUTPUT_LENGTH);
       
       return {
         stdout: stdout || null,
         stderr: stderr || null,
         compile_output: data.compile_output || null,
         status: data.status,
         time: data.time || null,
         memory: data.memory || null,
       };
     }
+    console.warn('[Execution] Judge0 CE returned an unexpected response shape. Falling back to Wandbox...');
   } catch (judge0Error) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/services/judge0Service.js` around lines 86 - 102, Add a warning
immediately before the Judge0 response-handling block falls through when
data.status.id is undefined, clearly indicating that Judge0 returned an
unexpected response payload and execution is falling back to Wandbox. Keep the
existing catch warning for request failures unchanged.

125-130: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider marking the maintenance error as retryable for callers.

The thrown maintenance error is a plain Error with only a message string. server/routes/execute.js passes it to next(err) with no special handling, so callers can only detect "retryable" failures by matching a substring in the message. Attach a machine-readable marker (e.g., err.statusCode = 503 or err.retryable = true) so the route and downstream error handling can respond with the correct HTTP status and support automated retry.

♻️ Proposed fix to mark the error as retryable
     if (isWandboxRuntimeFailure) {
-      throw new Error('Remote execution engine is currently undergoing maintenance. Please retry in a few moments.');
+      const maintenanceError = new Error('Remote execution engine is currently undergoing maintenance. Please retry in a few moments.');
+      maintenanceError.statusCode = 503;
+      maintenanceError.retryable = true;
+      throw maintenanceError;
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/services/judge0Service.js` around lines 125 - 130, Update the
maintenance error thrown in the isWandboxRuntimeFailure branch to include a
machine-readable retryable marker, such as statusCode 503 or retryable true,
while preserving the existing message and next(err) propagation through
execute.js.
server/routes/webhooks.js (1)

186-189: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Cover the changed no-destination response contract.

Line 186 changes /api/webhooks/room-event from HTTP 503 to HTTP 200 without dispatching an event. Add a regression test for the status and response body. Verify that callers do not rely on HTTP 503 to detect missing integration configuration. Keep a separate test for configured destination failures returning HTTP 502.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/routes/webhooks.js` around lines 186 - 189, Add regression coverage
for the no-destination branch in the room-event webhook handler, asserting HTTP
200 and the complete response body including success, empty dispatched results,
and the configuration message. Verify callers do not depend on HTTP 503 for
missing integrations, while preserving a separate test that configured
destination failures return HTTP 502.
src/hooks/useRoom.js (1)

119-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the redundant early-return branch that calls setState in the effect body.

When roomId or user becomes falsy, React runs the previous effect's cleanup (Lines 180-184) before this body executes. That cleanup already calls disconnect(), setSocket(null), and setIsConnected(false).

The branch is therefore redundant. It also reads socket, which is not in the dependency list at Line 185, so it can act on a stale value. The synchronous setSocket(null) call triggers the ESLint react-hooks/set-state-in-effect error reported for Line 123 and forces an extra render.

Delete the branch.

♻️ Proposed refactor
   useEffect(() => {
-    if (!roomId || !user) {
-      if (socket) {
-        socket.disconnect();
-        setSocket(null);
-      }
-      setIsConnected(false);
-      return;
-    }
+    if (!roomId || !user) return;
 
     const apiUrl = import.meta.env.VITE_API_URL || 'http://localhost:3001';
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks/useRoom.js` around lines 119 - 127, Remove the early-return branch
in the useRoom effect that checks !roomId or !user, including its socket
disconnect and state updates, so the effect no longer performs redundant
setState calls or reads stale socket state; rely on the existing cleanup for
disconnect and reset behavior.

Source: Linters/SAST tools

server/package.json (1)

30-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove unused Yjs dependencies. Both manifests add yjs and y-websocket, and the client also adds y-monaco, but the repository has no code that consumes them. Keep the CRDT/editor binding only if this PR lands it.

  • server/package.json#L30-L31: remove y-websocket and yjs.
  • package.json#L50-L54: remove y-monaco, y-websocket, or add the Yjs binding that uses them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/package.json` around lines 30 - 31, Remove the unused Yjs
dependencies: in server/package.json lines 30-31, remove y-websocket and yjs; in
package.json lines 50-54, remove y-monaco and y-websocket unless this PR adds
the Yjs binding that consumes them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@firestore.rules`:
- Around line 19-20: Replace the broad authenticated-user read rule for room
documents with a participant-only condition, and ensure private room credentials
and server-only hash fields are never exposed through that document. Update the
room-creation flow to write the join-time metadata (`name` and
`passwordProtected`) from the server into the existing `rooms/{roomId}/meta`
subcollection, preserving the narrower metadata read contract used for
inspection before joining.

In `@server/routes/rooms.js`:
- Around line 171-191: Update the POST /create handler to authenticate the
Firebase ID token using the existing verification pattern, derive the creator
UID from the verified token instead of req.body.createdBy, and apply the same
rate limiter used by /verify-password. Replace the rooms document set with merge
behavior by using create() so existing room IDs are never modified, and map the
ALREADY_EXISTS failure to HTTP 409.
- Around line 178-193: Update the room creation handler around the db guard to
return an HTTP 503 error response immediately when db is unavailable. Only
execute the rooms document write and return the existing 200 success response
when db is configured, preventing clients from treating an unpersisted room as
successfully created.

In `@server/services/socketService.js`:
- Around line 18-32: Secure the Socket.IO flow around the connection handler and
its join-room, code-update, stdin-update, roles-update, and chat-message
handlers: add io.use() Firebase ID-token verification, store the verified uid
and identity on socket.data, and require the client token from
src/hooks/useRoom.js with connect_error refresh handling. In join-room, load the
room document, require the verified uid in participantIds, and derive the role
from roles[uid] instead of payload values; reject unauthorized joins. Enforce
that resolved role for code, stdin, and role mutations/broadcasts, and construct
chat identity from the verified token rather than client-supplied
uid/displayName.
- Around line 167-178: Update the Firestore persistence block in the room-empty
handling flow to use a merge-enabled set operation so missing room documents are
created while preserving existing fields. Include a fresh Firestore Timestamp in
updatedAt, importing Timestamp from firebase-admin/firestore, and refresh this
field periodically for active rooms—not only when they become empty—so
roomCleanupService does not remove live sessions.

In `@src/hooks/useRoom.js`:
- Around line 219-234: Update the synchronization logic in useRoom’s
code-update, room-state, stdin-update handlers, and push useEffect to track when
code or stdin values were applied remotely; consume that marker in the effect to
skip emitting the corresponding remote-applied value while preserving local
edits and existing debounce behavior.

---

Outside diff comments:
In `@src/components/Editor/EditorPage.jsx`:
- Around line 1943-1961: Update both ChatPanel instances in EditorPage so each
receives key={room.roomId}, ensuring switching rooms remounts ChatPanel and
resets its local messages state.

---

Major comments:
In `@eslint.config.js`:
- Around line 24-30: Update the flat ESLint configuration after the existing
config entry with a later rule targeting src/hooks/__tests__ test files, and set
languageOptions.globals.vitest for that pattern so direct describe, it, expect,
and vi usage is recognized.

In `@firestore.rules`:
- Around line 56-58: Update the Firestore rules for the affected subcollections
around the `allow read/create/update/delete` statements so access requires room
participation, creation binds the document to its author using the field names
written by `useWebRTC.js` and the whiteboard component, and deletion is limited
to that author or the intended recipient. Apply the same restrictions to the
additional affected rule block.
- Around line 11-14: Remove the `allow read: if true` rule from the
`/users/{userId}` match block, preserving the existing owner-only access rule.
Update signup display-name uniqueness checks to use the
`displayNames/{normalizedName}` collection or a server-side Admin SDK lookup
rather than reading user documents publicly.

In `@server/middleware/rateLimiter.js`:
- Around line 4-5: Update getIpKey to derive the key from the client IP rather
than passing the request object to ipKeyGenerator: use req.ip as the fallback
input and normalize it through ipKeyGenerator(req.ip), preserving the actual
client-IP-based limiter bucket.

In `@server/services/judge0Service.js`:
- Around line 3-29: Update getJudge0Config so configured non-RapidAPI envUrl
values are preserved instead of falling back to the public Judge0 endpoint. Add
a custom-URL branch after the RapidAPI handling, returning envUrl with the
required submissions?wait=true path and JSON headers, while retaining the
existing RapidAPI and default public endpoint behavior.

In `@server/services/socketService.js`:
- Around line 5-7: Make room persistence durable by periodically flushing each
activeRooms entry when room.code differs from lastSavedCode, including code,
language, stdin, and updatedAt, then update lastSavedCode after a successful
write. Add a SIGTERM shutdown handler that flushes all rooms before exit, and
configure the Socket.IO Redis adapter with shared room-state storage so users
connected to different instances observe the same edits.
- Around line 149-182: Update the room membership flow around join-room and the
disconnect handler to track socket IDs per active user, adding and removing
socket.id from each user’s sockets Set and removing the user only when that Set
becomes empty. Before switching rooms, remove the socket from its previous
room’s membership and Socket.IO room so rejoining cannot leave stale presence or
subscriptions; preserve the existing empty-room persistence and cleanup only
after the last member is gone.
- Around line 94-114: Cap incoming code and stdin payloads before storing or
broadcasting them: define and reuse byte-size limits, including the proposed
MAX_STDIN_BYTES guard, and reject or truncate values exceeding the limits.
Update both the code-update and stdin-update handlers in the socket service, and
configure the Socket.IO server’s maxHttpBufferSize to enforce the
transport-level limit while preserving normal room updates.

In `@src/components/Auth/AuthModal.jsx`:
- Around line 75-86: Update the username uniqueness-check catch block in
AuthModal’s signup flow to fail closed: after logging the lookup error, stop
signup, clear the loading state, and surface an appropriate error instead of
continuing to create the account. Preserve the existing duplicate-name rejection
behavior and ensure createUserWithEmailAndPassword runs only when the lookup
completes successfully.

In `@src/components/Chat/ChatPanel.jsx`:
- Around line 46-66: Restore chat persistence by updating the server’s
chat-message relay in socketService to write each message to the room’s
Firestore messages subcollection before or alongside broadcasting it. In
ChatPanel’s mount effect, load recent messages for roomId from Firestore,
initialize messages with that history, and retain the existing socket listener
for new-message updates and deduplication.

In `@src/hooks/useRoom.js`:
- Around line 136-147: Update the socket join flow around the connect handler
and the effect at line 185 so room membership is emitted again when myRole
changes after roomData loads. Keep the initial connection behavior, but add a
role-reactive join-room emission using the existing payload and socket state,
ensuring the server receives the authoritative resolved role instead of
retaining the initial viewer value.
- Around line 356-361: Update the Socket.IO presence-update handling so the
server persists the current active-user count in the room document, preserving
the existing activeUsers field shape consumed by castVote and the
deadlock-resolution effect. Ensure the write occurs whenever presence changes
and that vote creation and quorum checks read this fresh server-derived value
instead of defaulting to an empty list.
- Around line 246-296: Update createRoom to derive a password hash and fresh
salt when trimmedPassword is present, then include passwordHash and passwordSalt
in the server room-creation payload over HTTPS; do not rely on the client-side
setDoc for these protected fields, since they must be written through the Admin
SDK. Ensure password-protected rooms persist both materials and unprotected
rooms retain the existing behavior.

In `@src/hooks/useWebRTC.js`:
- Around line 100-116: Update the signal handling in useWebRTC’s onSnapshot
callback to consume or deduplicate each signal document before creating a peer
or calling peer.signal(). Ensure documents replayed as added on listener
startup, including stale pre-disconnect signals, are ignored after prior
processing while allowing each current call-session signal to be applied once.

In `@vite.config.js`:
- Around line 20-26: Update the build configuration’s rolldownOptions by
removing the external entry for monaco-editor/esm/vs/editor/editor.api so Monaco
is bundled for browser production builds; if peer dependency resolution still
fails, add monaco-editor directly to the build inputs or dependencies using the
existing project convention.

---

Minor comments:
In `@package.json`:
- Line 11: Extend the lint script to lint server files alongside src while
excluding the React JSX/client pattern, and add a later ESLint configuration
block targeting server/**/*.js with CommonJS source type and Node globals.
Preserve the existing client/React configuration for src and ensure the server
override takes precedence.

In `@server/services/judge0Service.js`:
- Around line 6-7: Update the ESLint configuration for server-side JavaScript
files to include the Node globals environment, ensuring process in
judge0Service.js and other server files is recognized and no longer triggers
no-undef. Preserve the existing browser and ES2020 globals configuration for
non-server files.

In `@server/tests/cors.test.js`:
- Around line 15-18: Import the Vitest globals used by
server/tests/cors.test.js—describe, it, and expect—before the existing describe
block so the test file passes lint without relying on implicit globals.

In `@src/components/Auth/AuthModal.jsx`:
- Around line 77-79: Normalize the display name once in the AuthModal submission
flow, then reuse that trimmed value for the users query, updateProfile call, and
saveUser/displayNameLower derivation. Update the relevant variable and
references near getDocs and updateProfile so inputs such as surrounding
whitespace produce one canonical stored and queried name.

In `@src/components/Chat/ChatPanel.jsx`:
- Around line 80-81: Update the message-id assignment in ChatPanel to use
crypto.randomUUID() instead of the Math.random-based value, preserving the
resulting id’s use as the React key and deduplication key.

In `@src/hooks/useRoom.js`:
- Around line 173-178: Update the presence-update handling in useRoom so
remoteCursors is pruned against the current presence list, removing entries for
users who are no longer present while preserving active cursor entries. Keep the
existing cursor-move updates in the newSocket handler unchanged.

---

Nitpick comments:
In `@server/package.json`:
- Around line 30-31: Remove the unused Yjs dependencies: in server/package.json
lines 30-31, remove y-websocket and yjs; in package.json lines 50-54, remove
y-monaco and y-websocket unless this PR adds the Yjs binding that consumes them.

In `@server/routes/webhooks.js`:
- Around line 186-189: Add regression coverage for the no-destination branch in
the room-event webhook handler, asserting HTTP 200 and the complete response
body including success, empty dispatched results, and the configuration message.
Verify callers do not depend on HTTP 503 for missing integrations, while
preserving a separate test that configured destination failures return HTTP 502.

In `@server/services/judge0Service.js`:
- Around line 86-102: Add a warning immediately before the Judge0
response-handling block falls through when data.status.id is undefined, clearly
indicating that Judge0 returned an unexpected response payload and execution is
falling back to Wandbox. Keep the existing catch warning for request failures
unchanged.
- Around line 125-130: Update the maintenance error thrown in the
isWandboxRuntimeFailure branch to include a machine-readable retryable marker,
such as statusCode 503 or retryable true, while preserving the existing message
and next(err) propagation through execute.js.

In `@src/hooks/useRoom.js`:
- Around line 119-127: Remove the early-return branch in the useRoom effect that
checks !roomId or !user, including its socket disconnect and state updates, so
the effect no longer performs redundant setState calls or reads stale socket
state; rely on the existing cleanup for disconnect and reset behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4f139569-5efd-4b1a-9c2a-b854363b3ebd

📥 Commits

Reviewing files that changed from the base of the PR and between 35f1d76 and 9a28c3c.

⛔ Files ignored due to path filters (2)
  • package-lock.json is excluded by !**/package-lock.json
  • server/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (22)
  • .gitignore
  • eslint.config.js
  • firestore.rules
  • package.json
  • server/middleware/rateLimiter.js
  • server/package.json
  • server/routes/execute.js
  • server/routes/rooms.js
  • server/routes/webhooks.js
  • server/server.js
  • server/services/firebaseAdmin.js
  • server/services/judge0Service.js
  • server/services/roomCleanupService.js
  • server/services/socketService.js
  • server/tests/cors.test.js
  • src/components/Auth/AuthModal.jsx
  • src/components/Chat/ChatPanel.jsx
  • src/components/Editor/EditorPage.jsx
  • src/hooks/useRoom.js
  • src/hooks/useWebRTC.js
  • vite.config.js
  • vitest.config.js

Comment thread firestore.rules
Comment on lines +19 to +20
// Allow any authenticated user to read room documents (needed to inspect/join rooms)
allow read: if request.auth != null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift

Room reads now expose passwordHash, passwordSalt, and private room code.

Firestore rules are not filters. allow read: if request.auth != null grants every signed-in user the full room document, including passwordHash and passwordSalt.

The write rules on Lines 24-25, 30, 34, and 38 block clients from writing those fields, and server/routes/rooms.js Line 75 calls them "server-only hash fields". This read rule contradicts that design. Any signed-in user can now export the hash and salt of every room and run an offline attack against the scrypt hash.

The same rule also exposes code for every private room, so a user who does not know the password can read the room contents and bypass the password gate.

Restrict document reads to participants, and move the fields needed for "inspect before join" into the existing rooms/{roomId}/meta subcollection (Lines 95-98), which already has a narrower contract.

🔒 Proposed direction
-      // Allow any authenticated user to read room documents (needed to inspect/join rooms)
-      allow read: if request.auth != null;
+      // Participants only. Non-participants read rooms/{roomId}/meta to inspect a room
+      // before joining. Password hash/salt must never reach a client.
+      allow read: if request.auth != null
+                  && (
+                    request.auth.uid == resource.data.createdBy
+                    || request.auth.uid in resource.data.participantIds
+                  );

Write the join-time metadata (name, passwordProtected) to rooms/{roomId}/meta from the server when the room is created.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Allow any authenticated user to read room documents (needed to inspect/join rooms)
allow read: if request.auth != null;
// Participants only. Non-participants read rooms/{roomId}/meta to inspect a room
// before joining. Password hash/salt must never reach a client.
allow read: if request.auth != null
&& (
request.auth.uid == resource.data.createdBy
|| request.auth.uid in resource.data.participantIds
);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@firestore.rules` around lines 19 - 20, Replace the broad authenticated-user
read rule for room documents with a participant-only condition, and ensure
private room credentials and server-only hash fields are never exposed through
that document. Update the room-creation flow to write the join-time metadata
(`name` and `passwordProtected`) from the server into the existing
`rooms/{roomId}/meta` subcollection, preserving the narrower metadata read
contract used for inspection before joining.

Comment thread server/routes/rooms.js
Comment on lines +171 to +191
router.post('/create', async (req, res) => {
const { roomId, name, createdBy, isPrivate, passwordProtected, code, language } = req.body;
if (!roomId || !createdBy) {
return res.status(400).json({ error: 'roomId and createdBy are required.' });
}

try {
if (db) {
const now = new Date();
await db.collection('rooms').doc(roomId.trim()).set({
name: name || `Room ${roomId}`,
createdBy,
isPrivate: Boolean(isPrivate),
passwordProtected: Boolean(passwordProtected),
code: code || '',
language: language || 'python',
participantIds: [createdBy],
roles: { [createdBy]: 'host' },
createdAt: now,
updatedAt: now,
}, { merge: true });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift

Authenticate POST /create and stop merging into existing rooms.

This endpoint has no authentication and takes createdBy from the request body. It writes with the Admin SDK, so firestore.rules does not apply. The comment on Line 169 states this explicitly.

An attacker can send an existing roomId with their own createdBy. Because the write uses { merge: true }, the request replaces createdBy, roles, and participantIds, and it resets isPrivate and passwordProtected. The attacker becomes host of any room and can remove password protection. It also overwrites code and language, which destroys the collaborators' work.

Required changes:

  • Verify a Firebase ID token and derive createdBy from the verified uid.
  • Use create() instead of set(..., { merge: true }) so an existing room is never modified.
  • Apply a rate limiter, as /verify-password does.
🔒 Proposed fix
-router.post('/create', async (req, res) => {
-  const { roomId, name, createdBy, isPrivate, passwordProtected, code, language } = req.body;
-  if (!roomId || !createdBy) {
-    return res.status(400).json({ error: 'roomId and createdBy are required.' });
-  }
+router.post('/create', roomCreateLimiter, async (req, res) => {
+  const { roomId, name, isPrivate, passwordProtected, code, language } = req.body;
+  if (!roomId || typeof roomId !== 'string' || !roomId.trim()) {
+    return res.status(400).json({ error: 'roomId is required.' });
+  }
+
+  // Derive the owner from a verified ID token, never from the request body.
+  const idToken = (req.headers.authorization || '').replace(/^Bearer\s+/i, '');
+  if (!idToken) {
+    return res.status(401).json({ error: 'Authentication required.' });
+  }
+  let createdBy;
+  try {
+    const decoded = await getAuth(getApp()).verifyIdToken(idToken);
+    createdBy = decoded.uid;
+  } catch {
+    return res.status(401).json({ error: 'Invalid credentials.' });
+  }
 
   try {
-    if (db) {
-      const now = new Date();
-      await db.collection('rooms').doc(roomId.trim()).set({
+    const now = new Date();
+    await db.collection('rooms').doc(roomId.trim()).create({
         name: name || `Room ${roomId}`,
         createdBy,
         isPrivate: Boolean(isPrivate),
         passwordProtected: Boolean(passwordProtected),
         code: code || '',
         language: language || 'python',
         participantIds: [createdBy],
         roles: { [createdBy]: 'host' },
         createdAt: now,
         updatedAt: now,
-      }, { merge: true });
-    }
+    });

create() rejects with ALREADY_EXISTS. Map that error to a 409 response.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/routes/rooms.js` around lines 171 - 191, Update the POST /create
handler to authenticate the Firebase ID token using the existing verification
pattern, derive the creator UID from the verified token instead of
req.body.createdBy, and apply the same rate limiter used by /verify-password.
Replace the rooms document set with merge behavior by using create() so existing
room IDs are never modified, and map the ALREADY_EXISTS failure to HTTP 409.

Comment thread server/routes/rooms.js
Comment on lines +178 to +193
if (db) {
const now = new Date();
await db.collection('rooms').doc(roomId.trim()).set({
name: name || `Room ${roomId}`,
createdBy,
isPrivate: Boolean(isPrivate),
passwordProtected: Boolean(passwordProtected),
code: code || '',
language: language || 'python',
participantIds: [createdBy],
roles: { [createdBy]: 'host' },
createdAt: now,
updatedAt: now,
}, { merge: true });
}
return res.status(200).json({ success: true, roomId });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win

Return an error when db is not configured.

If Firebase Admin has no credentials, db is null, the if (db) block is skipped, and the handler still returns 200 { success: true }. The client fallback in src/hooks/useRoom.js (Lines 288-299) treats that response as success, calls setRoomId(id), and stores the id in localStorage. The user then joins a room that does not exist, and the subsequent Firestore snapshot never resolves.

Return 503 when db is unavailable.

🐛 Proposed fix
   try {
-    if (db) {
+    if (!db) {
+      console.error('[rooms] create room requested but Firestore is not configured');
+      return res.status(503).json({ error: 'Room service unavailable.' });
+    }
     const now = new Date();
-      await db.collection('rooms').doc(roomId.trim()).set({
+    await db.collection('rooms').doc(roomId.trim()).set({
       ...
-      }, { merge: true });
-    }
+    }, { merge: true });
     return res.status(200).json({ success: true, roomId });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/routes/rooms.js` around lines 178 - 193, Update the room creation
handler around the db guard to return an HTTP 503 error response immediately
when db is unavailable. Only execute the rooms document write and return the
existing 200 success response when db is configured, preventing clients from
treating an unpersisted room as successfully created.

Comment on lines +18 to +32
io.on('connection', (socket) => {
let currentRoomId = null;
let currentUser = null;

logger.info(`[Socket] Client connected: ${socket.id}`);

// ─── Join Room ────────────────────────────────────────────────────────────
socket.on('join-room', async ({ roomId, user, role }) => {
if (!roomId || !user) return;

currentRoomId = roomId;
currentUser = user;
socket.join(roomId);

logger.info(`[Socket] User ${user.displayName} (${user.uid}) joined room ${roomId}`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift

Add authentication and authorization to the Socket.IO server.

There is no io.use() middleware and no token verification. Every handler trusts the client payload. This defeats the room password flow and the Firestore role model.

Concrete consequences:

  • Line 26 accepts any roomId and any user object. A client that knows a room id joins the room and receives room-state with the full code (Lines 85-90) plus every later code-update, stdin-update, and chat-message. The password check in server/routes/rooms.js is never consulted, so it is client-enforced only.
  • Line 71 takes role from the client. A viewer declares itself host.
  • Lines 94 and 106 apply code and stdin with no role check. firestore.rules Line 38 forbids viewers from writing code, but this path bypasses the rules and then persists the result with Admin credentials at Line 169. The read-only guarantee is void.
  • Line 142 relays the client-supplied uid and displayName verbatim. Any client can impersonate another user in chat.
  • Line 136 broadcasts an arbitrary roles map with no ownership check.

Verify a Firebase ID token in a connection middleware, derive uid from the token, and resolve the role from the Firestore room document instead of the payload.

🔒 Proposed direction
+const { getAuth } = require('firebase-admin/auth');
+
 function initSocketServer(server, allowedOrigins) {
   const io = new Server(server, { cors: { ... } });
 
+  // Reject unauthenticated connections before any handler runs.
+  io.use(async (socket, next) => {
+    try {
+      const token = socket.handshake.auth?.token;
+      if (!token) return next(new Error('unauthorized'));
+      const decoded = await getAuth().verifyIdToken(token);
+      socket.data.uid = decoded.uid;
+      socket.data.email = decoded.email;
+      return next();
+    } catch {
+      return next(new Error('unauthorized'));
+    }
+  });
+
   io.on('connection', (socket) => {

Then in join-room, load the room document, confirm socket.data.uid is in participantIds, and read the role from roles[socket.data.uid]. Reject the join otherwise. In code-update, stdin-update, and roles-update, check the resolved role before mutating or broadcasting. In chat-message, replace message.uid and message.displayName with the verified identity.

The client must pass the token. In src/hooks/useRoom.js Line 130, add auth: { token: await auth.currentUser.getIdToken() } and refresh it on connect_error.

Also applies to: 71-71, 94-114, 136-146

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/services/socketService.js` around lines 18 - 32, Secure the Socket.IO
flow around the connection handler and its join-room, code-update, stdin-update,
roles-update, and chat-message handlers: add io.use() Firebase ID-token
verification, store the verified uid and identity on socket.data, and require
the client token from src/hooks/useRoom.js with connect_error refresh handling.
In join-room, load the room document, require the verified uid in
participantIds, and derive the role from roles[uid] instead of payload values;
reject unauthorized joins. Enforce that resolved role for code, stdin, and role
mutations/broadcasts, and construct chat identity from the verified token rather
than client-supplied uid/displayName.

Comment on lines +167 to +178
try {
if (firestoreDb) {
await firestoreDb.collection('rooms').doc(roomId).update({
code: room.code,
language: room.language,
stdinValue: room.stdin,
});
logger.info(`[Socket] Room ${roomId} successfully saved to Firestore.`);
}
} catch (err) {
logger.error(`[Socket] Failed to save empty room ${roomId} to Firestore: ${err.message}`);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win

update() rejects when the room document is missing, and updatedAt is never written.

Two defects in this persistence block:

  1. Line 169 calls update(). The Admin SDK rejects update() with NOT_FOUND if the document does not exist. join-room accepts any roomId, and Lines 43-49 tolerate a missing document by seeding empty state. When such a room empties, the write rejects, the catch at Line 176 only logs, and the session code is lost. Use set(..., { merge: true }).

  2. The write omits updatedAt. server/services/roomCleanupService.js Line 94 queries rooms where updatedAt < cutoff and then calls recursiveDelete on each match. A room that is actively used through Socket.IO never refreshes updatedAt, so the cleanup job deletes live rooms and their subcollections after ROOM_STALE_HOURS. That is silent data loss.

🐛 Proposed fix
             if (firestoreDb) {
-              await firestoreDb.collection('rooms').doc(roomId).update({
-                code: room.code,
-                language: room.language,
-                stdinValue: room.stdin,
-              });
+              await firestoreDb.collection('rooms').doc(roomId).set(
+                {
+                  code: room.code,
+                  language: room.language,
+                  stdinValue: room.stdin,
+                  updatedAt: Timestamp.now(),
+                },
+                { merge: true }
+              );
               logger.info(`[Socket] Room ${roomId} successfully saved to Firestore.`);
             }

Add const { Timestamp } = require('firebase-admin/firestore'); at the top. Also refresh updatedAt periodically while the room is active, not only when it empties.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/services/socketService.js` around lines 167 - 178, Update the
Firestore persistence block in the room-empty handling flow to use a
merge-enabled set operation so missing room documents are created while
preserving existing fields. Include a fresh Firestore Timestamp in updatedAt,
importing Timestamp from firebase-admin/firestore, and refresh this field
periodically for active rooms—not only when they become empty—so
roomCleanupService does not remove live sessions.

Comment thread src/hooks/useRoom.js
Comment on lines 219 to +234
useEffect(() => {
if (!roomId || !user || !roomData) return;
if (!roomId || !user || !socket || !isConnected) return;
if (!isEditor) return;
const timer = setTimeout(() => {
updateDoc(doc(db, 'rooms', roomId), {
socket.emit('code-update', {
roomId,
code,
language,
});
socket.emit('stdin-update', {
roomId,
stdin: stdinValue,
_lastEditor: user.uid,
updatedAt: serverTimestamp(),
}).catch(() => {});
});
}, 300);
return () => clearTimeout(timer);
}, [code, language, stdinValue, roomId, user, isEditor, roomData]);
}, [code, language, stdinValue, roomId, user, isEditor, socket, isConnected]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🔴 Critical | 🏗️ Heavy lift

Remote code updates re-emit themselves, creating an echo loop.

The code-update handler at Line 164 calls setCode(rCode). code is a dependency of this push effect (Line 234). So every remote update schedules a local emit of the same content 300 ms later.

The server stores that emit at socketService.js Line 98 and re-broadcasts it to all other clients, whose handlers call setCode and emit again. One keystroke by one user produces one emit from every other member of the room. The amplification grows with the room size and does not settle while anyone keeps typing.

The room-state handler at Line 153 causes the same effect on join: a user who joins immediately echoes the whole room code back to the server.

Track whether the current value came from a remote apply, and skip the emit in that case.

🐛 Proposed fix
+  const applyingRemoteRef = useRef(false);
     newSocket.on('room-state', ({ code: rCode, language: rLang, stdin: rStdin, activeUsers: rUsers }) => {
+      applyingRemoteRef.current = true;
       if (rCode !== undefined) setCode(rCode);
       ...
     });
 
     newSocket.on('code-update', ({ code: rCode, language: rLang }) => {
+      applyingRemoteRef.current = true;
       if (rCode !== undefined) setCode(rCode);
       if (rLang) setLanguage(rLang);
     });
   useEffect(() => {
     if (!roomId || !user || !socket || !isConnected) return;
     if (!isEditor) return;
+    if (applyingRemoteRef.current) {
+      // This render came from a remote update. Do not send it back.
+      applyingRemoteRef.current = false;
+      return;
+    }
     const timer = setTimeout(() => {

Apply the same guard in the stdin-update handler at Line 169.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks/useRoom.js` around lines 219 - 234, Update the synchronization
logic in useRoom’s code-update, room-state, stdin-update handlers, and push
useEffect to track when code or stdin values were applied remotely; consume that
marker in the effect to skip emitting the corresponding remote-applied value
while preserving local edits and existing debounce behavior.

@vijaypatil477 vijaypatil477 removed their assignment Aug 1, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@server/tests/cors.test.js`:
- Around line 15-18: Update the test containing the `it` and `expect` calls to
resolve Vitest globals by importing them from Vitest, or configure Vitest
globals for this test scope; ensure ESLint recognizes both symbols without
changing the test behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8a2a1941-f97c-464f-9457-4fa8f5c54790

📥 Commits

Reviewing files that changed from the base of the PR and between 9a28c3c and e82e9bf.

📒 Files selected for processing (2)
  • server/server.js
  • server/tests/cors.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • server/server.js

Comment thread server/tests/cors.test.js
Comment on lines +15 to 18
it('rejects requests without an Origin header', async () => {
const response = await request(app).options('/api/execute');

expect(response.status).toBe(403);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Fix the undefined Vitest globals.

ESLint reports it and expect as undefined. Import them from Vitest, or configure Vitest globals for this test scope. Otherwise, the lint step can fail.

+import { expect, it } from 'vitest';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('rejects requests without an Origin header', async () => {
const response = await request(app).options('/api/execute');
expect(response.status).toBe(403);
import { expect, it } from 'vitest';
it('rejects requests without an Origin header', async () => {
const response = await request(app).options('/api/execute');
expect(response.status).toBe(403);
🧰 Tools
🪛 ESLint

[error] 15-15: 'it' is not defined.

(no-undef)


[error] 18-18: 'expect' is not defined.

(no-undef)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/tests/cors.test.js` around lines 15 - 18, Update the test containing
the `it` and `expect` calls to resolve Vitest globals by importing them from
Vitest, or configure Vitest globals for this test scope; ensure ESLint
recognizes both symbols without changing the test behavior.

Source: Linters/SAST tools

@vijaypatil477
vijaypatil477 merged commit 12742f0 into main Aug 2, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gssoc:approved GSSoC '26 Approved issue level:critical GSSoC '26 Critical difficulty issue quality:exceptional Exceptional code quality contribution type:bug Vulnerability or logical bug fixes type:performance Performance and latency optimizations type:testing Integration and unit test coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants